Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stat Collector #1007

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Stat Collector #1007

wants to merge 7 commits into from

Conversation

SteveMicroNova
Copy link
Contributor

@SteveMicroNova SteveMicroNova commented Dec 12, 2024

What does this change intend to accomplish?

Add an optional user statistics survey to help us decide what to spend our devtime on, and potentially help find bugs and improve support

Currently this is a draft that only contains the script that collects the stats, this will eventually contain a good means of invoking the script, a means of sending the data upstream to home base, and a portion of the UI that allows a user to activate the functionality in general

Checklist

  • Have you tested your changes and ensured they work?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • If applicable, have you updated the CHANGELOG?
  • Does your submission pass linting & tests? You can test on localhost using ./scripts/test
  • If this is a UI change, have you tested it across multiple browser platforms on both desktop and mobile?

Comment on lines +167 to +182
@api.get('/collected_stats')
@api.get('/collected_stats/')
def get_collected_stats():
"""Get the current contents of statcollector output file after filtering out any empty stream data"""
survey = dict(statcollector.UsageSurveySchema.load_from_disk())
return {header: data for header, data in survey.items() if data != statcollector.StreamUsageSchema()}


@api.post('/collected_stats')
@api.post('/collected_stats/')
def post_collected_stats():
"""Send the contents of the statcollector output file to AmpliPi devs"""
survey = statcollector.UsageSurveySchema.load_from_disk()
survey.phone_home()


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These do not belong at the top of this file
I do not know where they belong, but it isn't the top of the top
They stay here in the meantime because it's easier to not need to scroll too much to edit imports if need be during devwork

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These stats are effectively metadata about the api. Putting them in the API feels weird. That said keeping them outside of the api may make accessing them a little more awkward.

if len(matches) != 0: # If there is at least one playing match, calculate runtime
self.current_runtime = self.current_runtime + timediff.seconds

self.peak_runtime = max(self.peak_runtime, self.current_runtime) if self.current_runtime < 86400 else 86400
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no real reason to limit this to 24 hours, I just had the feeling that more than 24 hours would lead to outliers and this is a method of negating that. It might be worthwhile to remove the limit just so we can see what sort of lifespan that long-lived stream errors depend on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants